Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Python codegen: use | for union types #3956

Closed
wants to merge 5 commits into from
Closed

Python codegen: use | for union types #3956

wants to merge 5 commits into from

Conversation

emilk
Copy link
Member

@emilk emilk commented Oct 23, 2023 •
edited
Loading

What

This replaces Union[A, B, C] with A | B | C in our generated Python code.

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested demo.rerun.io (if applicable)
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

@emilk emilk added 🐍 Python API Python logging API exclude from changelog PRs with this won't show up in CHANGELOG.md labels Oct 23, 2023
@emilk emilk marked this pull request as draft October 23, 2023 12:49
@emilk emilk changed the title Improve generated Python code Python codegen: use | for union types Oct 25, 2023
@emilk
Copy link
Member Author

emilk commented Oct 25, 2023 •
edited
Loading

just py-build && python -m rerun fails with:

Traceback (most recent call last):
  File "/Users/emilk/.pyenv/versions/3.8.12/lib/python3.8/runpy.py", line 185, in _run_module_as_main
    mod_name, mod_spec, code = _get_module_details(mod_name, _Error)
  File "/Users/emilk/.pyenv/versions/3.8.12/lib/python3.8/runpy.py", line 144, in _get_module_details
    return _get_module_details(pkg_main_name, error)
  File "/Users/emilk/.pyenv/versions/3.8.12/lib/python3.8/runpy.py", line 111, in _get_module_details
    __import__(pkg_name)
  File "/Users/emilk/code/rerun/rerun/rerun_py/rerun/__init__.py", line 26, in <module>
    sys.modules["rerun"] = __import__("rerun")
  File "/Users/emilk/code/rerun/rerun/rerun_py/rerun_sdk/rerun/__init__.py", line 126, in <module>
    from ._image import ImageEncoded, ImageFormat
  File "/Users/emilk/code/rerun/rerun/rerun_py/rerun_sdk/rerun/_image.py", line 10, in <module>
    from ._log import AsComponents, ComponentBatchLike
  File "/Users/emilk/code/rerun/rerun/rerun_py/rerun_sdk/rerun/_log.py", line 8, in <module>
    from . import components as cmp
  File "/Users/emilk/code/rerun/rerun/rerun_py/rerun_sdk/rerun/components/__init__.py", line 5, in <module>
    from .annotation_context import (
  File "/Users/emilk/code/rerun/rerun/rerun_py/rerun_sdk/rerun/components/annotation_context.py", line 13, in <module>
    from .. import datatypes
  File "/Users/emilk/code/rerun/rerun/rerun_py/rerun_sdk/rerun/datatypes/__init__.py", line 6, in <module>
    from .annotation_info import (
  File "/Users/emilk/code/rerun/rerun/rerun_py/rerun_sdk/rerun/datatypes/annotation_info.py", line 99, in <module>
    AnnotationInfoArrayLike = AnnotationInfo | Sequence[AnnotationInfoLike]
TypeError: unsupported operand type(s) for |: 'type' and '_GenericAlias'

I'm in the normal env:

❯ python --version
Python 3.8.12

Relevant diffs:

image

image

I don't see anything obviously wrong

@emilk
Copy link
Member Author

emilk commented Oct 25, 2023

Antoine explained it to me: | is not available in Python 3.8. We can still use | in function signatures etc, because python ignores them during execution, but we cannot use them in type definitions, because those are actually executed.

So this will have to wait until we drop support for 3.8, if ever

@emilk emilk closed this Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md 🐍 Python API Python logging API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@emilk